-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
[rb] allow setting socket timeout and interval from http client #16472
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
PR Code Suggestions ✨Explore these optional code suggestions:
|
That would be better, but I don't have it in my head how we'd implement it. It would be a breaking change. |
5173a9f
to
e1a06da
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather have those options separate from HTTP because they really have nothing to do with HTTP clients. We can add a new class ClientConfig and allow to pass it to the Driver constructor. This won't be a breaking change and spare us from deprecating these fields in HTTP in the future.
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
I thought the plan was to remove the ability to pass in a custom http client entirely, in which case it wouldn't matter what we added since it would all end up private api? Trying to figure out what the middle ground is to get what I want without needing to do all of 16269 |
Converting to draft until after #16486 |
User description
🔗 Related Issues
fixes #15620
💥 What does this PR do?
Allows setting socket timeout and interval values on the default http client.
🔧 Implementation Notes
This is an alternative to #16053
I'm not sure "default http config" is the perfect place for this, kind of wish we just had a config file like others, but the superclass gets the http client instance, so easy enough to grab it and send it to the ws socket class.
💡 Additional Considerations
🔄 Types of changes
PR Type
Enhancement
Description
Allow configuring socket timeout and interval via HTTP client
Pass HTTP client instance to BiDi and WebSocket connections
Replace hardcoded timeout constants with configurable values
Add socket timeout parameter to test environment setup
Diagram Walkthrough
File Walkthrough
bidi.rb
Add HTTP client parameter to BiDi initialization
rb/lib/selenium/webdriver/bidi.rb
initialize
method to accepthttp
parameterhttp
parameter toWebSocketConnection
constructorwebsocket_connection.rb
Make socket timeout and interval configurable
rb/lib/selenium/webdriver/common/websocket_connection.rb
initialize
to accept optionalhttp
parametersocket_timeout
andsocket_interval
from HTTP client withdefaults
RESPONSE_WAIT_TIMEOUT
andRESPONSE_WAIT_INTERVAL
constants with instance variables
wait
method to use configurable timeout and interval valuesbidi_bridge.rb
Pass HTTP client to BiDi initialization
rb/lib/selenium/webdriver/remote/bidi_bridge.rb
@http
client instance toBiDi.new
constructordefault.rb
Add socket timeout and interval configuration
rb/lib/selenium/webdriver/remote/http/default.rb
socket_timeout
andsocket_interval
as accessor attributessocket_timeout
andsocket_interval
parameters toinitialize
methodwith defaults (30 and 0.1)
test_environment.rb
Enable BiDi and add socket timeout test support
rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb
ENV['WEBDRIVER_BIDI']
create_driver!
to accept and handlesocket_timeout
optionhttp_client
parameter to driver creation methodsdefault_spec.rb
Add socket timeout and interval test coverage
rb/spec/unit/selenium/webdriver/remote/http/default_spec.rb
socket_timeout
(30) andsocket_interval
(0.1)
initialization
websocket_connection.rbs
Add socket timeout and interval type signatures
rb/sig/lib/selenium/webdriver/common/websocket_connection.rbs
@socket_interval
(float) and@socket_timeout
(int) instance variables
default.rbs
Add socket timeout and interval type signatures
rb/sig/lib/selenium/webdriver/remote/http/default.rbs
socket_timeout
(int) andsocket_interval
(float) accessors